Fix: bump adm-zip to ^0.6.0 to clear GHSA-xcpc-8h2w-3j85 (CVE-2026-39244) - #663

Please ensure you have read the contribution guide before creating a pull request.

Link to Issue or Description of Change

1. Link to an existing issue (if applicable):
   Closes: adm-zip range ^0.5.17 pins consumers to a version with an unpatched DoS (GHSA-xcpc-8h2w-3j85) google/adk-js#601
   Related: adm-zip range ^0.5.17 pins consumers to a version with an unpatched DoS (GHSA-xcpc-8h2w-3j85) google/adk-js#601
2. Or, if no issue exists, describe the change:
   Problem: Every published @google/adk resolves adm-zip to a version affected by
   GHSA-xcpc-8h2w-3j85 / CVE-2026-39244
   — "Crafted ZIP file triggers 4GB memory allocation", CWE-400 + CWE-789, High, CVSS 3.1
   score 7.5 (AV:N/AC:L/PR:N/UI:N/S:U/C:N/I:N/A:H). adm-zip's zipEntry.js called
   Buffer.alloc(_centralHeader.size) using the attacker-controlled uncompressed-size field
   from the ZIP central directory, before decompressing and before CRC validation, so a
   ~120-byte archive can force a ~4 GB allocation and crash the process.

The advisory range is <0.6.0, and the fix only exists in adm-zip@0.6.0 — a
semver-major release that is unreachable from the ^0.5.17 range this repo declared.
That is why downstream consumers could not remediate it themselves: overrides (flat and
nested), npm update, npm dedupe and a from-scratch lockfile resolve all leave 0.5.x
in the tree when the dependency crosses a workspace edge. The publisher has to bump it.

Solution: Declare "adm-zip": "^0.6.0" in both manifests that require it and
regenerate the lockfile.

• core/package.json — dependencies (runtime, shipped in @google/adk; this is the
  edge that reaches consumers).
• dev/package.json — devDependencies (build-time only, used by dev/build.js).
• core/package.json — the @types/adm-zip devDependency is deleted. The bump is what
  makes it dead weight: 0.5.17 published no declarations, while 0.6.0 declares
  "types": "types.d.ts", so TypeScript resolves the bundled file and never reads
  node_modules/@types/adm-zip. DefinitelyTyped never published a 0.6.x, so keeping it
  would pin stale 0.5-era types beside adm-zip@^0.6.0.

Both are needed: leaving either behind keeps a <0.6.0 requirement in the workspace, and
npm would resolve a second, still-vulnerable adm-zip node to satisfy it — reintroducing
the advisory. adm-zip stays in the section it was already in; neither declaration moved.

Compatibility evidence — no source adaptation was required. adm-zip has exactly two
call sites in the repo (grep -rn "adm-zip\|AdmZip" --include='*.ts' --include='*.js'):

Two test files also build fixtures with new AdmZip(), addFile(), toBuffer() and
assignment to entry.entryName; all still behave identically. 0.6.0 additionally ships
its own types.d.ts, and both call sites typecheck against it.

Behavioural deltas in 0.6.0 that could have mattered, and why they do not here: CRC
validation for data-descriptor archives now trusts the central-directory CRC instead of
throwing DESCRIPTOR_* (strictly more archives load; nothing in this repo asserts on those
errors), the entry table is now built with Object.create(null) (prototype-pollution
hardening — a free win for the untrusted skill-loading path), extractAllTo no longer
aborts when utimesSync fails (strictly more forgiving for the build script), and
engines.node moves >=12.0 → >=14.0 (no effect: dev/build.js already targets
node16).

Reachability, stated honestly: the vulnerable path is only reached by consumers that
load skill bundles through loadSkillFromZipBuffer, or by the build script unpacking the
ADK Web release archive. For most users this is supply-chain hygiene — a npm audit finding
they cannot clear — rather than a live exploit. That lowers the urgency; it does not remove
the need to fix, since the publisher is the only party who can.

Not a breaking change for @google/adk. adm-zip is an internal implementation
detail: it is not re-exported and appears in no public type of @google/adk. A
semver-major bump of an internal dependency is not a semver-major event for this package.

Collision check (required before implementing): gh pr list --repo AmaadMartin/adk-js --state open --limit 1000 returned 561 open PRs; none bumps adm-zip. The three that
touch adjacent ground were inspected with gh pr diff --name-only: #183 and #345 both
remove a stale "dev": true flag from the node_modules/adm-zip lockfile entry — that
flag is already absent on current main, so their hunk no longer applies and does not
overlap these three; #567 edits dev/package.json to declare @types/js-yaml, a different
key. No stacking was needed.

Deliberately out of scope (each would enlarge a security diff or conflict on the
lockfile): the "dev"-flag lockfile hygiene above, npm audit fix (the tree has 44 other
advisories), any other dependency bump, an engines field, and a hand-written zip-bomb
regression test — that last one would have to byte-patch the ZIP central-directory size
field, coupling an adk-js test to adm-zip internals for no signal the version pin does
not already give.

Testing Plan

Please describe the tests that you ran to verify your changes. This is required for all PRs that are not small documentation or typo fixes.
Unit Tests:
[x] I have added or updated unit tests for my change.
[x] All unit tests pass locally.

No source lines changed (git diff --stat is core/package.json | 3 +--,
dev/package.json | 2 +-, package-lock.json | 23 +++-----), so there is no new code to
cover and therefore no new tests and no coverage delta — the repo-wide V8 thresholds in
vitest.config.ts (statements 86, branches 87, functions 88, lines 86) are untouched
because no core/src/** or dev/src/** line moved. No existing test file was modified.

The evidence that the bump is inert is the existing suites that perform a real in-memory
zip round trip through adm-zip@0.6.0:

The load-bearing cases inside describe('loadSkillFromZipBuffer'):

• loads a benign archive with all resource trees — builds SKILL.md + references/ +
  assets/ + scripts/ with addFile() + toBuffer(), reads it back and asserts the
  parsed frontmatter, instructions and all three resource maps.
• rejects the whole archive for the dangerous entry %s (/etc/passwd, ../evil.txt,
  references/../../esc.txt) — these assign entry.entryName after addFile() (which
  would otherwise canonicalise the name away) and then toBuffer(). They assert the thrown
  message contains the byte-exact name, so their passing is direct proof that 0.6.0 still
  preserves a hand-written entryName through the write/parse round trip. Had 0.6.0
  changed name handling, this is where it would surface.
• reports the dangerous entry even when SKILL.md is absent, and
  rejects the non-bare skill name %s (../evil, a/b, ..) — the zip-slip and bare-name
  guards live in adk-js (isDangerousZipEntryName, isBareSkillName), not in adm-zip,
  and are unperturbed.

Proof the tests can fail (mutation testing against adm-zip@0.6.0; both mutations were
reverted and the working tree verified clean afterwards):

1. Neutered the zip-slip guard — isDangerousZipEntryName replaced with
   return entryName === 'MUTATION_NEVER_MATCHES'; → 4 failures, e.g.
   AssertionError: expected [Function] to throw error including 'Dangerous zip entry ignored: ../evil.…' but got 'SKILL.md not found in zipped filesyst…'.
2. Broke the payload round trip — const data = entry.getData(); replaced with
   const data = Buffer.from('MUTATED'); in loadZipDir → 2 failures across both
   files: AssertionError: expected 'MUTATED' to be 'ref content'.

Proof that deleting @types/adm-zip is a compiler no-op. I ran npm run ts:check twice
on this branch, once with the package absent and once after reinstalling it with
npm install --no-save @types/adm-zip@0.5.8. Both runs report the same 281 errors and
the sorted error sets diff clean, so nothing depended on the DefinitelyTyped
declarations. npm run build — which is tsc --emitDeclarationOnly for core, and the
real typecheck of loader.ts — also passes with the package absent from node_modules,
emitting core/dist/types/skills/loader.d.ts.

npm run ts:check is not green on this branch, and is not green on main either — the
281 errors above are all of the form core/dist/types/... is not assignable to core/src/...
(duplicate type identity between the built core/dist declarations and core/src), and
none is in an adm-zip call site. Also proven unrelated to the bump: swapping
node_modules/adm-zip back to 0.5.17 produces a byte-identical 281-error set.
ts:check is not part of the CI workflow. Both adm-zip call sites compile cleanly.

One unit:dev case, cli_create_test.ts > should handle Vertex AI selection with gcloud defaults, fails on a workstation that exports GOOGLE_CLOUD_PROJECT /
GOOGLE_CLOUD_LOCATION: the test mocks execSync but createAgent prefers the real
environment, so the assertion sees the developer's own project instead of gcloud-project.
Under env -u GOOGLE_CLOUD_PROJECT -u GOOGLE_CLOUD_LOCATION all 223 pass. That is an
environment leak in an unrelated test, not a regression from this bump, and is left alone
here.

Manual End-to-End (E2E) Tests:

1. Both manifests declare the patched range:

   grep -n '"adm-zip"' core/package.json dev/package.json
   #   core/package.json:63:    "adm-zip": "^0.6.0",
   #   dev/package.json:48:    "adm-zip": "^0.6.0",
   

2. The lockfile resolves to the patched version, from the public registry:

   node -p "require('./package-lock.json').packages['node_modules/adm-zip'].version"   # 0.6.0
   npm ls adm-zip
   #   ├─┬ @google/adk-devtools@1.5.0 -> ./dev
   #   │ └── adm-zip@0.6.0
   #   └─┬ @google/adk@1.5.0 -> ./core
   #     └── adm-zip@0.6.0 deduped
   

   integrity was cross-checked against npm view adm-zip@0.6.0 dist.integrity and matches
   (sha512-XleryMhbuksdKtofnWZ9Sk+…). Exactly one hoisted node, no duplicate tree, and the
   entry still carries no "dev" flag (correct — core needs it at runtime).
3. The advisory is gone (npm audit reads the lockfile; the acceptance criterion):

   # before: {"info":0,"low":7,"moderate":22,"high":13,"critical":3,"total":45}
   # after:  {"info":0,"low":7,"moderate":22,"high":12,"critical":3,"total":44}
   # adm-zip entry after the bump: null
   # advisories removed: ['adm-zip'];  advisories added: []
   

   The high count drops by exactly one and no other advisory changed — this bump fixes the
   one thing it claims to and nothing else.
4. npm run build — exercises the second call site for real. dev/build.js
unzipFile() is the only use of the path-string constructor and of extractAllTo in the
   repo. It downloaded the ADK Web v1.0.0 release archive and unpacked it with 0.6.0:

   [ADK Build] Downloaded and cached ADK Web v1.0.0.
   [ADK Build] Extracting ADK Web assets to dist/browser...
   [ADK Build] ADK Web assets successfully populated.
   

   107 files landed in dev/dist/browser, .version reads v1.0.0.
5. The DefinitelyTyped package is gone from the installed tree, so the build above
   typechecked against the bundled declarations only:

   ls node_modules/@types/adm-zip     # No such file or directory
   node -p "require('adm-zip/package.json').types"   # types.d.ts
   

6. npm run lint, npm run format:check and npm run docs:check all exit 0.

CI: run-tests is green on ubuntu-latest, macos-latest and windows-latest. The first
windows run failed on
core/test/code_executors/unsafe_local_code_executor_test.ts > should execute shell code and return stdout with Error: Test timed out in 5000ms — a known windows-only timing flake in
a test unrelated to zip handling (the same commit passed on the other two runners, and 2689
of 2690 tests passed on windows). It passed on re-run with no code change.

Checklist

[x] I have read the CONTRIBUTING.md document.
[x] I have performed a self-review of my own code.
[x] I have commented my code, particularly in hard-to-understand areas.
[x] I have added tests that prove my fix is effective or that my feature works.
[x] New and existing unit tests pass locally with my changes.